Add ForceMove option on vacate parameters #489

riccardomarinelli · 2024-10-10T05:44:05Z

Description

Adds ForceMove parameter to vacate parametes, in order to choose primitive vacate or standard vacate

Related Issues

Rel: https://elasticco.atlassian.net/browse/INFRA-3057
Rel: https://elasticco.atlassian.net/browse/INFRA-2823

Motivation and Context

When a vacate plan fails due to Primitive vacate failure ( with error InfrastructureFailure:CopyDataFailure), we want to be able to retry with Standad vacate instead of Primitive vacate from automation tools like Fleetctl in order to try to reduce the number of SDH issues created.

How Has This Been Tested?

This has been tested locally from Fleetctl https://github.com/elastic/fleetctl/pull/195, testing the cloud-sdk-go changes with this line in go.mod replace github.com/elastic/cloud-sdk-go => ../cloud-sdk-go

Types of Changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Refactoring (improves code quality but has no user-facing effect)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation

Readiness Checklist

My code follows the code style of this project
My change requires a change to the documentation
I have updated the documentation accordingly
I have added tests to cover my changes
All new and existing tests passed

gheorghepucea

Thanks for adding this. I have two questions.

gheorghepucea · 2024-10-14T07:16:30Z

pkg/api/platformapi/allocatorapi/vacate.go

@@ -391,6 +393,7 @@ func newMoveClusterParams(params *VacateClusterParams) (*platform_infrastructure
 		platform_infrastructure.NewMoveClustersParams().
 			WithAllocatorDown(params.AllocatorDown).
 			WithMoveOnly(params.MoveOnly).
+			WithForceMove(params.ForceMove).


Is this needed here? I would remove it because I think it alters the soteria behaviour i.e. currently we:

Issue a request to validate the move request validate_only=true

Use the params returned to issue the actual vacate

Now, if we set the force_move flag in the validation API, the admin console will respect what u asked.
So in case of soteria, what would be the behaviour? which value will the force_move param have?

If I remove this one, This is the result ( see logs https://platform-logging.kb.eu-west-1.aws.qa.cld.elstc.co/app/r/s/1ElD8 )

Testing with Force_move: true in Fleetctl code:
Our of 3 log entries,

The first one will have the correct value forceMove=[true]:

Request to move clusters: (allocatorId=[gi-5926589073801201158], authz=[Some(1)], moveRequest=[None], maybeClusterType=[None], forceUpdate=[false], allocatorDown=[None], validateOnly=[true], moveOnly=[true], forceMove=[true])

The second and the third one will have the wrong value forceMove=[false]:

Request to move clusters: (allocatorId=[gi-5926589073801201158], authz=[Some(1)], moveRequest=[Some(MoveClustersRequest(Some(List(MoveElasticsearchClusterConfiguration(List(44e610693f534648a7ccacedcb16c65b),None))),None,None,None,None))], maybeClusterType=[None], forceUpdate=[false], allocatorDown=[Some(false)], validateOnly=[true], moveOnly=[true], forceMove=[false])

Request to move clusters: (allocatorId=[gi-5926589073801201158], authz=[Some(1)], moveRequest=[Some(MoveClustersRequest(Some(List(MoveElasticsearchClusterConfiguration(List(44e610693f534648a7ccacedcb16c65b),Some(TransientElasticsearchPlanConfiguration(Some(PlanStrategy(None,Some(GrowShrinkStrategyConfig()),None,None)),Some(EsPlanControlConfiguration(Some(32768),Some(5),None,Some(List(AllocatorMoveRequest(gi-5926589073801201158,None,Some(false)))),Some(true),None,None,Some(false),None,None,Some(false),Some(3),Some(300),Some(false),None,Some(false),Some(false),Some(false),Some(false),Some(false),None)),None,None,None))))),None,None,None,None))], maybeClusterType=[Some(ElasticsearchClusterType)], forceUpdate=[false], allocatorDown=[Some(false)], validateOnly=[false], moveOnly=[true], forceMove=[false])

Testing with Force_move: true in Fleetctl code:

all the logs entries will have forceMove=[false] see https://platform-logging.kb.eu-west-1.aws.qa.cld.elstc.co/app/r/s/NtaT0

gheorghepucea · 2024-10-14T07:21:03Z

pkg/api/platformapi/allocatorapi/vacate.go

@@ -127,6 +127,7 @@ func moveNodes(id string, params *VacateParams, p *pool.Pool) ([]pool.Validator,
 			WithAllocatorID(id).
 			WithMoveOnly(params.MoveOnly).
 			WithContext(api.WithRegion(context.Background(), params.Region)).
+			WithForceMove(params.ForceMove).


Same here, won't this alter the behaviour of the soteria flow? for example to always set force_move to true?

If I remove this one, This is the result ( see logs https://platform-logging.kb.eu-west-1.aws.qa.cld.elstc.co/app/r/s/g99SA )

Testing with Force_move: true in Fleetctl code:
Our of 3 log entries,

The first one will have the wong value forceMove=[false]:

Request to move clusters: (allocatorId=[gi-5926589073801201158], authz=[Some(1)], moveRequest=[None], maybeClusterType=[None], forceUpdate=[false], allocatorDown=[None], validateOnly=[true], moveOnly=[true], forceMove=[false])

The second and the third one will have the correct value forceMove=[true]:

Request to move clusters: (allocatorId=[gi-5926589073801201158], authz=[Some(1)], moveRequest=[Some(MoveClustersRequest(Some(List(MoveElasticsearchClusterConfiguration(List(44e610693f534648a7ccacedcb16c65b),None))),None,None,None,None))], maybeClusterType=[None], forceUpdate=[false], allocatorDown=[Some(false)], validateOnly=[true], moveOnly=[true], forceMove=[true])

Request to move clusters: (allocatorId=[gi-5926589073801201158], authz=[Some(1)], moveRequest=[Some(MoveClustersRequest(Some(List(MoveElasticsearchClusterConfiguration(List(44e610693f534648a7ccacedcb16c65b),Some(TransientElasticsearchPlanConfiguration(Some(PlanStrategy(None,Some(GrowShrinkStrategyConfig()),None,None)),Some(EsPlanControlConfiguration(Some(32768),Some(5),None,Some(List(AllocatorMoveRequest(gi-5926589073801201158,None,Some(false)))),Some(true),None,None,Some(false),None,None,Some(false),Some(3),Some(300),Some(false),None,Some(false),Some(false),Some(false),Some(false),Some(false),None)),None,None,None))))),None,None,None,None))], maybeClusterType=[Some(ElasticsearchClusterType)], forceUpdate=[false], allocatorDown=[Some(false)], validateOnly=[false], moveOnly=[true], forceMove=[false])

Testing with Force_move: true in Fleetctl code:

all the logs entries will have forceMove=[false] see see logs https://platform-logging.kb.eu-west-1.aws.qa.cld.elstc.co/app/r/s/se28H

riccardomarinelli · 2024-10-14T10:37:32Z

As a last attempt added ForceMove as a Plan Override in the last commit and tested but also was unsuccessful

riccardomarinelli · 2024-10-14T10:38:46Z

Also tested directly with curl, but the plans in adminconsole still had force_move: true

 ~ curl -X POST -H "Authorization: ApiKey $QA_API" -H "Content-type: application/json" -d @payload.json "https://admin.qa.cld.elstc.co/api/v1/regions/gcp-us-central1/platform/infrastructure/allocators/gi-4896651367850012401/clusters/elasticsearch/_move?allocator_down=false&force_move=false"
{
  "failures" : {
    "elasticsearch_clusters" : [
    ],
    "kibana_clusters" : [
    ],
    "apm_clusters" : [
    ],
    "appsearch_clusters" : [
    ],
    "enterprise_search_clusters" : [
    ]
  },
  "moves" : {
    "elasticsearch_clusters" : [
      {
        "cluster_id" : "44e610693f534648a7ccacedcb16c65b"
      }
    ],
    "kibana_clusters" : [
    ],
    "apm_clusters" : [
    ],
    "appsearch_clusters" : [
    ],
    "enterprise_search_clusters" : [
    ]
  }
}%
➜  ~ curl -X POST -H "Authorization: ApiKey $QA_API" -H "Content-type: application/json" -d @payload.json "https://admin.qa.cld.elstc.co/api/v1/regions/gcp-us-central1/platform/infrastructure/allocators/gi-4896651367850012401/clusters/elasticsearch/_move?allocator_down=false"
{
  "failures" : {
    "elasticsearch_clusters" : [
    ],
    "kibana_clusters" : [
    ],
    "apm_clusters" : [
    ],
    "appsearch_clusters" : [
    ],
    "enterprise_search_clusters" : [
    ]
  },
  "moves" : {
    "elasticsearch_clusters" : [
      {
        "cluster_id" : "44e610693f534648a7ccacedcb16c65b"
      }
    ],
    "kibana_clusters" : [
    ],
    "apm_clusters" : [
    ],
    "appsearch_clusters" : [
    ],
    "enterprise_search_clusters" : [
    ]
  }
}%

riccardomarinelli · 2024-10-14T10:40:03Z

Moving this back as a Draft and will create a Bug Jira as commented with Gheorge Phucea

riccardomarinelli added 2 commits October 10, 2024 13:41

Add ForceMove option on vacate parameters

ba19c5b

Update moveParams to add ForceMove

5ea6794

riccardomarinelli added the enhancement New feature or request label Oct 14, 2024

riccardomarinelli marked this pull request as ready for review October 14, 2024 04:56

riccardomarinelli requested a review from a team as a code owner October 14, 2024 04:56

Remove uneeded entries and Update vacate Tests

799ce06

riccardomarinelli force-pushed the Add_ForceMove_option_on_vacate_parameters branch from dd1c07b to 799ce06 Compare October 14, 2024 05:21

riccardomarinelli requested review from claudia-correia and gheorghepucea October 14, 2024 05:25

riccardomarinelli added 2 commits October 14, 2024 13:46

Add previously removed force-move entries

7d3cc67

Remove platform_infrastructure.NewMoveClustersByTypeParams entry

9a53596

gheorghepucea reviewed Oct 14, 2024

View reviewed changes

riccardomarinelli requested a review from Z3r0Sum October 14, 2024 07:51

Add FroceMove as PlanOverride

d8b9dc8

riccardomarinelli marked this pull request as draft October 14, 2024 10:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ForceMove option on vacate parameters #489

Add ForceMove option on vacate parameters #489

riccardomarinelli commented Oct 10, 2024 •

edited

Loading

gheorghepucea left a comment

gheorghepucea Oct 14, 2024

riccardomarinelli Oct 14, 2024 •

edited

Loading

gheorghepucea Oct 14, 2024

riccardomarinelli Oct 14, 2024 •

edited

Loading

riccardomarinelli commented Oct 14, 2024

riccardomarinelli commented Oct 14, 2024

riccardomarinelli commented Oct 14, 2024

Add ForceMove option on vacate parameters #489

Are you sure you want to change the base?

Add ForceMove option on vacate parameters #489

Conversation

riccardomarinelli commented Oct 10, 2024 • edited Loading

Description

Related Issues

Motivation and Context

How Has This Been Tested?

Types of Changes

Readiness Checklist

gheorghepucea left a comment

Choose a reason for hiding this comment

gheorghepucea Oct 14, 2024

Choose a reason for hiding this comment

riccardomarinelli Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

gheorghepucea Oct 14, 2024

Choose a reason for hiding this comment

riccardomarinelli Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

riccardomarinelli commented Oct 14, 2024

riccardomarinelli commented Oct 14, 2024

riccardomarinelli commented Oct 14, 2024

riccardomarinelli commented Oct 10, 2024 •

edited

Loading

riccardomarinelli Oct 14, 2024 •

edited

Loading

riccardomarinelli Oct 14, 2024 •

edited

Loading